CSHARP-5943: Initial small AGENTS.md file for the C# driver#1934
CSHARP-5943: Initial small AGENTS.md file for the C# driver#1934ajcvickers wants to merge 3 commits intomongodb:mainfrom
Conversation
|
For reviewers, see notes and discussion here: https://jira.mongodb.org/browse/DRIVERS-3428 |
There was a problem hiding this comment.
Pull request overview
Adds an initial agent-focused repository guide for the MongoDB C# driver, outlining the tech stack, repo layout, common commands, and test environment variables.
Changes:
- Introduces a new
Agents.mdwith a high-level overview of the driver and repository structure. - Documents build/test commands and a matrix of environment variables for optional test suites.
- Notes commit/PR message conventions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Tech Stack | ||
| - .NET library projects producing NuGet packages | ||
| - Multi-targeted to various .NET versions from .NET Framework 4.7.2 up | ||
| - xUnit + FluentAssertions for testing |
There was a problem hiding this comment.
This part about FluentAssertions.. I think we need to discuss about it in the team. Sometime ago we had a discussion about using the basic assertions against the fluent ones.
There was a problem hiding this comment.
I have added FluentAssertions to my list of things to bring up in retrospective. For now, I think we should leave it--Claude explicitly called out that it was useful.
There was a problem hiding this comment.
Hm... just to leave the record: I do remember discussion about FluentAssertion turned to be commercial, and we should either stop using it, or pin the version and never bump it until it possible. But I do not remember if we really made any decisions. We also discussed possibility to switch to Shouldly.
There was a problem hiding this comment.
Exactly what you said. I also think that at the time we didn't decide which direction to take though.
There was a problem hiding this comment.
I think this is the only more "contentious point". Should we leave it out and let the AI check the follow the conventions that are already in the file? Or check similar tests/tests for similar classes.
|
|
||
| ## Commit and PR Conventions | ||
|
|
||
| - Commit and PR messages start with a JIRA number: `CSHARP-1234: Description` |
There was a problem hiding this comment.
This is true only for the first commit in a branch, doesn't it?
In my file I also have that branch names contain the JIRA ticket number. And also that it can use the ticket number from a branch to look up context from JIRA when needed.
There was a problem hiding this comment.
I will clarify that it is the first commit.
Sometimes branch names don't match--even Robert had names with "b" appended or similar occasionally. That's why I didn't add it, but I can, and we can see if any exceptions cause real issues.
| - Multi-targeted to various .NET versions from .NET Framework 4.7.2 up | ||
| - xUnit + FluentAssertions for testing | ||
|
|
||
| ## Project Structure |
There was a problem hiding this comment.
Something that I found useful was also to give the path to the specification repo that I have locally. It's super useful for certain kind of feature implementation.
There was a problem hiding this comment.
Interesting. I think we would have to standardize on the location to put that here, but it could be a good candidate for each of our user-specific agents file.
There was a problem hiding this comment.
I agree, for sure we should not standardize the location, but it's a good add.
| | X.509 authentication | `MONGO_X509_CLIENT_CERTIFICATE_PATH`, `MONGO_X509_CLIENT_CERTIFICATE_PASSWORD` | | ||
| | PLAIN authentication | `PLAIN_AUTH_TESTS_ENABLED` | | ||
| | SOCKS5 proxy | `SOCKS5_PROXY_SERVERS_ENABLED` | | ||
|
|
There was a problem hiding this comment.
Another line I have:
"Do not strip BOMs: Source files may have a UTF-8 BOM (U+FEFF). Prefer using the Edit tool over Write to avoid accidentally removing it."
(not sure if it's still an issue)
There was a problem hiding this comment.
What issues have we had with BOMs?
There was a problem hiding this comment.
On some files both Claude and OpenCode were removing BOMs, so I was getting a change on the first line of some files.
| ## Tech Stack | ||
| - .NET library projects producing NuGet packages | ||
| - Multi-targeted to various .NET versions from .NET Framework 4.7.2 up | ||
| - xUnit + FluentAssertions for testing |
There was a problem hiding this comment.
I think this is the only more "contentious point". Should we leave it out and let the AI check the follow the conventions that are already in the file? Or check similar tests/tests for similar classes.
|
I don't think the FluentAssertions line should be contentious. It's accurate for the current code base and Claude indicated it was useful. If we start transitioning away from using FluentAssertions, then we should remove it and tell it to favor whatever we are moving to. |
See notes in DRIVERS-3428.